Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Downloader rewriter config has all_blocked_message #12997

Closed

Conversation

illicitonion
Copy link
Contributor

@illicitonion illicitonion commented Feb 11, 2021

This allows for the config author to specify a useful error message to
adorn the user-facing error, so that if the user is unaware or forgets
about the config, they get a more clear error message than:

ERROR: java.io.IOException: Error downloading [] to /some/path

or

Error in download: java.io.IOException: Cache miss and no url specified

Currently there is a log line like:

INFO: Rewritten [https://doesnotexist.com/beep] as []

printed, but this is often quite far from the error - this puts all the
information in one place.

If you don't configure an all_blocked_message, this now looks like:

ERROR: java.io.IOException: Cache miss and no url specified - Configured URL rewriter blocked all URLs: [https://doesnotexist.com/beep]

And if you do, it will look like something like:

ERROR: java.io.IOException: Cache miss and no url specified - Configured URL rewriter blocked all URLs: [https://doesnotexist.com/beep] - See https://mycorp.com/bazel-rewriter-issue for more details.

@google-cla google-cla bot added the cla: yes label Feb 11, 2021
@illicitonion
Copy link
Contributor Author

This depends on #12989

@aiuto aiuto added the team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. label Feb 22, 2021
@aiuto aiuto requested a review from coeuvre February 22, 2021 16:43
This allows for the config author to specify a useful error message to
adorn the user-facing error, so that if the user is unaware or forgets
about the config, they get a more clear error message than:
```
ERROR: java.io.IOException: Error downloading [] to /some/path
```
or
```
Error in download: java.io.IOException: Cache miss and no url specified
```

Currently there is a log line like:
```
INFO: Rewritten [https://doesnotexist.com/beep] as []
```

printed, but this is often quite far from the error - this puts all the
information in one place.

If you don't configure an all_blocked_message, this now looks like:

```
ERROR: java.io.IOException: Cache miss and no url specified - Configured URL rewriter blocked all URLs
```

And if you do, it will look like something like:

```
ERROR: java.io.IOException: Cache miss and no url specified - Configured URL rewriter blocked all URLs - See https://mycorp.com/bazel-rewriter-issue for more details.
```
@illicitonion
Copy link
Contributor Author

I rebased as the dependent PR got merged :)

Copy link
Member

@coeuvre coeuvre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR!

@bazel-io bazel-io closed this in 63bc1c7 Feb 26, 2021
@illicitonion illicitonion deleted the downloader-error-message branch February 26, 2021 12:37
philwo pushed a commit that referenced this pull request Mar 15, 2021
This allows for the config author to specify a useful error message to
adorn the user-facing error, so that if the user is unaware or forgets
about the config, they get a more clear error message than:
```
ERROR: java.io.IOException: Error downloading [] to /some/path
```
or
```
Error in download: java.io.IOException: Cache miss and no url specified
```

Currently there is a log line like:
```
INFO: Rewritten [https://doesnotexist.com/beep] as []
```

printed, but this is often quite far from the error - this puts all the
information in one place.

If you don't configure an all_blocked_message, this now looks like:

```
ERROR: java.io.IOException: Cache miss and no url specified - Configured URL rewriter blocked all URLs: [https://doesnotexist.com/beep]
```

And if you do, it will look like something like:

```
ERROR: java.io.IOException: Cache miss and no url specified - Configured URL rewriter blocked all URLs: [https://doesnotexist.com/beep] - See https://mycorp.com/bazel-rewriter-issue for more details.
```

Closes #12997.

PiperOrigin-RevId: 359730842
philwo pushed a commit that referenced this pull request Mar 15, 2021
This allows for the config author to specify a useful error message to
adorn the user-facing error, so that if the user is unaware or forgets
about the config, they get a more clear error message than:
```
ERROR: java.io.IOException: Error downloading [] to /some/path
```
or
```
Error in download: java.io.IOException: Cache miss and no url specified
```

Currently there is a log line like:
```
INFO: Rewritten [https://doesnotexist.com/beep] as []
```

printed, but this is often quite far from the error - this puts all the
information in one place.

If you don't configure an all_blocked_message, this now looks like:

```
ERROR: java.io.IOException: Cache miss and no url specified - Configured URL rewriter blocked all URLs: [https://doesnotexist.com/beep]
```

And if you do, it will look like something like:

```
ERROR: java.io.IOException: Cache miss and no url specified - Configured URL rewriter blocked all URLs: [https://doesnotexist.com/beep] - See https://mycorp.com/bazel-rewriter-issue for more details.
```

Closes #12997.

PiperOrigin-RevId: 359730842
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants